Skip to content

Simplify session timeout modal presenter#7985

Merged
aduth merged 3 commits intomainfrom
aduth-session-timeout-modal-presenter
Mar 15, 2023
Merged

Simplify session timeout modal presenter#7985
aduth merged 3 commits intomainfrom
aduth-session-timeout-modal-presenter

Conversation

@aduth
Copy link
Copy Markdown
Contributor

@aduth aduth commented Mar 14, 2023

Extracted from #7966

🛠 Summary of changes

Simplifies the behavior of the session timeout modal presenter:

  • Flatten separate and largely repeated classes FullySignedInModalPresenter and PartiallySignedInModalPresenter to a single SessionTimeoutModalPresenter
    • Name presenter such that it's obvious its purpose in handling session timeout behaviors
  • Move view rendering responsibilities out of presenter class and into view partial, limiting scope of the presenter class to handle differences between "fully" and "partially" signed-in users
  • Use standard design system buttons implemented through ButtonComponent
  • Avoid unclear abbreviation "sr" in favor of live_region_ qualifier describing its context in a ARIA live region, which notably can benefit more than just screen readers
  • Avoid setting initial expiration time in modal content, expecting that this will be assigned by the session timeout JavaScript [1] [2]

📜 Testing Plan

Confirm no regressions in the behavior of the session timeout modal, including specifically:

  • Differences between "partially" and "fully" signed in (i.e. before and after MFA authenticated)
  • "Stay signed in" and "Sign out" buttons work as expected
  • Modal text counts down

It's easier to test by setting a very low session timeout in local config/application.yml:

session_timeout_in_minutes: 1

Copy link
Copy Markdown
Contributor

@zachmargolis zachmargolis left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM

aduth and others added 3 commits March 14, 2023 16:45
changelog: Internal, Refactoring, Simplify logic for session timeout modal behavior
See: https://github.com/18F/identity-idp/pull/7985/files#r1136148250
Co-Authored-By: Zach Margolis <zachmargolis@users.noreply.github.com>
@aduth aduth force-pushed the aduth-session-timeout-modal-presenter branch from 0a76851 to 857e81f Compare March 14, 2023 20:48
@aduth aduth merged commit 244a837 into main Mar 15, 2023
@aduth aduth deleted the aduth-session-timeout-modal-presenter branch March 15, 2023 14:40
svalexander pushed a commit that referenced this pull request Mar 16, 2023
* Simplify session timeout modal presenter

changelog: Internal, Refactoring, Simplify logic for session timeout modal behavior

* Mark translations as used

See: https://github.com/18F/identity-idp/pull/7985/files#r1136148250
Co-Authored-By: Zach Margolis <zachmargolis@users.noreply.github.com>

* Rename locale_scope as translation_scope

See: https://github.com/18F/identity-idp/pull/7985/files#r1136148250

---------

Co-authored-by: Zach Margolis <zachmargolis@users.noreply.github.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants